-
Notifications
You must be signed in to change notification settings - Fork 14.4k
[ARM] Allow FP reg conversion when copying Sx to Dx #147559
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
This allows copying float value to Dx reg using inline asm, e.g: ``` float a; void b() { register float d1 asm("d1") = a; asm("" ::"r"(d1)); } ```
@llvm/pr-subscribers-backend-arm Author: None (eleviant) ChangesThis allows copying float value to Dx reg using inline asm, e.g:
Full diff: https://github.com/llvm/llvm-project/pull/147559.diff 2 Files Affected:
diff --git a/llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp b/llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
index 50217c3a047df..751f06c4eadf9 100644
--- a/llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
+++ b/llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
@@ -743,6 +743,9 @@ void ARMBaseInstrInfo::copyPhysReg(MachineBasicBlock &MBB,
Opc = ARM::VMOVSR;
else if (ARM::DPRRegClass.contains(DestReg, SrcReg) && Subtarget.hasFP64())
Opc = ARM::VMOVD;
+ else if (ARM::DPRRegClass.contains(DestReg) &&
+ ARM::SPRRegClass.contains(SrcReg) && Subtarget.hasFP64())
+ Opc = ARM::VCVTDS;
else if (ARM::QPRRegClass.contains(DestReg, SrcReg))
Opc = Subtarget.hasNEON() ? ARM::VORRq : ARM::MQPRCopy;
diff --git a/llvm/test/CodeGen/ARM/copy-reg-vcvt.ll b/llvm/test/CodeGen/ARM/copy-reg-vcvt.ll
new file mode 100644
index 0000000000000..fa93c3b1aae8c
--- /dev/null
+++ b/llvm/test/CodeGen/ARM/copy-reg-vcvt.ll
@@ -0,0 +1,17 @@
+; RUN: llc -filetype=asm -O3 %s -o - | FileCheck %s
+; CHECK: vldr s0, [r0]
+; CHECK-NEXT: vcvt.f64.f32 d1, s0
+
+target datalayout = "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64"
+target triple = "armv8a-unknown-linux-gnueabihf"
+
+@a = local_unnamed_addr global float 0.000000e+00, align 4
+
+; Function Attrs: mustprogress noimplicitfloat nounwind
+define void @_Z1bv() local_unnamed_addr {
+entry:
+ %0 = load float, ptr @a, align 4
+ tail call void asm sideeffect "", "{d1}"(float %0)
+ ret void
+}
+
|
Why do you think the compiler should do this? It's not what gcc does. And in general implicitly performing floating-point conversions seems like a bad idea. |
@efriedma-quic Well, the inline assembly in summary actually instructs to put float value into 64-bit FP reg and that's exactly what I did. GCC seems to put value into s1 instead of d1, which seems confusing. Compiling the same with clang results in undefined behavior in release build w/o asserts, (for me it consumes all available memory and crashes). What's the sane thing to do here in your opinion? Emit an error and exit? Doing what GCC does? |
There are two things wrong in the original testcase:
An error seems fine for a mismatch like this. Doing the same thing as gcc is also probably okay (putting the float value in the lower half of d1, which is s2). In any case, we probably should handle this during SelectionDAG, in ARMTargetLowering::getRegForInlineAsmConstraint or something like that. |
This allows copying float value to Dx reg using inline asm, e.g: